fix(security): second ReDoS sweep (#174) - #175

Fixes #174, the second ReDoS sweep found while verifying #173.

How I measured

Each site was timed in Node at 20k and 40k characters. A ×4 jump per doubling means quadratic.

• Every rewrite has an equivalence test against the old regex: seeded random strings over the characters that matter (3,000–10,000 per function) plus hand-picked cases.
• Every rewrite also has an adversarial case of 100k characters or more, with a time bound.

Items

1. Unauthenticated email check and body cap (packages/api)

• isEmailAddress (src/email.ts) checks the length (254) first. It then accepts exactly what /^[^\s@]+@[^\s@]+\.[^\s@]+$/ accepted, in one pass: one @, a dot inside the domain, no whitespace.
• The old regex took 917 ms on a@ + 40k dots, and password-reset.ts ran it before the length cap.
• invites.ts and password-reset.ts both use the new check, and the EMAIL regex is gone.
• jsonBody (the public token routes: reset request, inspect and complete, invite inspect and accept) reads at most JSON_BODY_MAX_BYTES = 64 KiB from the stream. Above that it answers 413 payload_too_large, both when content-length says so and when a chunked body runs past the cap.

Verified in test/public-input.test.ts:

• equivalence over every string of up to 6 characters drawn from a . @ space, plus samples;
• 100k adversarial strings in under 100 ms;
• the real route answers 400 on a 60k-dot address in under 500 ms (the old code took about 2 s) and 202 on a valid one;
• 413 for an oversized body, both with content-length and streamed;
• a body exactly at the cap is accepted.

2. Redirect paths (packages/core-ts/src/redirects.ts, packages/schedules/src/feed-check.ts)

• normalisePath and joinTarget use trimEndChar. The old trims took 621 ms on 40k slashes.
• ancestorPaths starts its walk below the first //. The contract's from pattern allows no empty segment, so an ancestor holding // can never match. A run of slashes used to yield one useless ancestor per slash.
• urlPath in feed-check uses an inline loop, since schedules does not depend on core-ts.

Verified:

• test/redos-174.test.ts: equivalence for normalisePath and joinTarget; ancestorPaths equals the old list filtered to valid froms; matchRedirect cases; 100k slash runs.
• schedules/test/xml-scan.test.ts: urlPath.

3. Docs Worker isPagePath (apps/docs/src/worker/paths.ts)
Replaced with lastIndexOf(".") <= lastIndexOf("/"). The old check took 622 ms on 40k dots.
Verified: apps/docs/test/paths.test.ts, with 5,000 random paths and 100k dots.

4. Lazy scans, measured one by one:

Verified:

• core-ts/test/redos-174.test.ts fuzzes 8,000 fence strings, including \r, and 5,000 url( strings. It also runs a whole checkSvg document with url( plus 20k spaces in under 500 ms (attributes are read from a 64 KiB window, so the document case stays below it) and searchText on 100k adversarial characters.
• schedules/test/xml-scan.test.ts fuzzes CDATA (3,000), elements (6,000) and media urls (8,000), all with 100k cases.

5. skills.ts frontmatter (packages/agent-runtime/src/frontmatter-lines.ts)

• Both line regexes are replaced. /^\s+([^:]+):\s*(.*)$/ was 650 ms on 40k spaces, and both \s*(.*)$ tails backtracked before a lone \r.
• The parsers give the same groups, including the regex's backtracking cases: an indent that runs up to : gives a one-space key, and a line terminator after the value fails the line.
• Skills come from the operator's skills directory (loadSkills in cloudhal mcp and cloudhal studio, or bundled into a Worker), so they are not caller input today. Fixed anyway.

Verified: test/frontmatter-lines.test.ts, with 10,000 random lines including \r and 
, and 100k cases.

6. Editor trims (packages/ui/src/blocks/editor/markdown-blocks.ts)
/\n*$/ becomes trimEndChar(…, "\n"). The old trim took 644 ms on 40k inner newlines.
Verified: test/markdown-blocks-redos.test.tsx, with equivalence on 5,000 strings and mergeBlocks with a 100k-newline block. The existing codec tests still pass. Correction: in the first commit this file was named .test.ts, which packages/ui/vitest.config.ts (test/**/*.test.tsx) never collected, so it had not run. Round 2 renames it; it runs and passes (2 tests).

Done-when items

• AGENTS.md hard rule 13 and a CONTRIBUTING bullet: regexes over external input must be linear, with the patterns to avoid and what to use instead.
• Changeset redos-sweep-174.md, patch, for api, core-ts, schedules, agent-runtime and cli. @cloudhal/ui and the docs app are private; the editor fix ships in the CLI's Studio.

Checked and left alone:

• Studio's roles.ts email check: it already checks the length first, so it is bounded at 254.
• MAIL_ADDRESS in cli/config.ts and cloudflare/mail.ts: operator configuration only.
• In searchText: the directive and inline-code patterns (measured linear) and the entity, URL and whitespace replaces.
• In svg.ts: the entity and attribute regexes. They are anchored, or read from a 64 KiB window.

Checks run locally (serially)

• Single files first: api/test/public-input.test.ts, core-ts/test/redos-174.test.ts, schedules/test/xml-scan.test.ts, agent-runtime/test/frontmatter-lines.test.ts + skills.test.ts, ui/test/markdown-editor-codec.test.tsx (the new ui test did not run in round 1; see item 6), docs/test/paths.test.ts + worker.test.ts: all green.
• Then turbo run test --concurrency=1: docs (3 files), agent-runtime (11), schedules (11), ui (16), astro-renderer (49), api (40): all green.
• core-ts: 59 of 61 files passed. The two D1-heavy files failed only with network errors (EADDRNOTAVAIL, fetch failed, one hook timeout): this machine ran out of local ports. Rerun on its own, list-values-query.test.ts passed 18/18.
• Left to CI: the D1 and Durable Object halves of store.test.ts. Its better-sqlite3 slice passed (156 tests; the other 281 belong to the two halves CI runs). The D1 and Durable Object halves need more connections than this machine can recycle, so CI runs them on Linux.
• Static checks: pnpm lint (0 errors), pnpm size:check, pnpm design:lint and turbo run typecheck (20/20) are green.

Review round 2 (second commit)

R1. ancestorPaths is bounded for many short segments (core-ts/src/redirects.ts)

• When the path is longer than REDIRECT_FROM_MAX_LENGTH (1024, the contract's maxLength for from), the walk starts at the longest ancestor within that length; then the // cut applies.
• No valid redirect's result changes: a longer ancestor can never be a from.
• "/a".repeat(50_000) gives 512 ancestors totalling under 1024 × 513 characters, in under 100 ms. It used to give about 50,000 ancestors, quadratic in size, JSON-encoded into one SQL parameter on every 404.
• Fuzzed paths up to about 1,800 characters equal the old list filtered to valid froms, including the length cap.

R2. Directive lines stay on one line (core-ts/src/search-rows.ts)

• The pattern is now DIRECTIVE_LINE: /^:{2,}[a-z][\w-]*(\{[^\n\r\u2028\u2029]*\})?[ \t]*$/gim.
• Under m, every line terminator starts a line, and [^\n]* ran across \r. "::a{}x\r" × 14,000 took 1,168 ms; it now takes 0.4 ms.
• Behaviour change: a directive whose braces span a lone \r or U+2028/2029 is no longer read as one directive. Markdown from Studio and agents is \n-only.
• Verified: fuzzed equivalence with the old regex on \n-only input (4,000 strings), and a searchText case on 100k characters of CR lines.

R3. stripFences keeps the next \n (core-ts/src/search-text-scan.ts)

• The next \n position is kept and recomputed only once an opener passes it. With no \n left, no later opener can have a body, so the scan stops.
• Verified: 100k cases with \r line ends, with U+2028 line ends, and with one \n at the very end. The 8,000-string fuzz still passes.

R4. mediaUrlValues keeps the next > and the next url (schedules/src/xml-scan.ts)

• forwardFinder keeps its last indexOf answer until a query passes it.
• Verified: 100k cases for '<enclosure url="a" ' × n, '<enclosure>' × n, and tags followed by a 50k CDATA section with the > after it.
• The previous xml-scan.ts fails the first of these (298 ms against the 100 ms bound). The 8,000-string fuzz still passes.

R5. Every new test file is collected (checked with vitest list <file> in each package):

Checks for round 2:

• Single files first: redos-174, xml-scan, markdown-blocks-redos.test.tsx.
• Then turbo run test --concurrency=1: schedules (11 files), ui (17, one more than round 1: the renamed test), astro-renderer (49) and api (40), all green.
• core-ts without store.test.ts: 60 files, 660 tests, all green. list-values-query passed this time.
• Left to CI again: the D1 and Durable Object halves of store.test.ts. They exceed this machine's local ports.
• pnpm lint (0 errors), pnpm size:check, pnpm design:lint and turbo run typecheck (20/20) are green.

Review round 3 (third commit, before 0.6.5)

1. Timing bounds follow the repo convention.

• Every new timing assertion is now 1.5 s on 200k-character inputs, as in Review follow-ups: MCP OAuth step-up scope set (phase 2) and CI/docs housekeeping #168's tests (core-ts/test/names.test.ts).
• Inputs are larger (400k–1M characters) where the replaced regex grew slowly: CSS comments, CDATA, <item>, the media url cases. The old code still takes seconds on every case.
• public-input.test.ts warms the app with a valid request first, then times two adversarial addresses against 1.5 s: 60k dots (under the body cap, answered 400; the old regex took about 2 s) and 200k dots (answered 413; the old code had no cap and took about 23 s).

2. The from length is counted in code points. The contract's maxLength (1024) is checked by ajv in code points, so ancestorPaths now caps at 2 × 1024 UTF-16 units, still a constant. A from of / plus 600 astral characters (601 code points, 1201 units) matches ${from}/x, and still matches when the request path is longer than the cap. Under the round-2 cap it would not have matched.

3. A // at the cut. The // cut is computed on the whole path before truncating, so the first ancestor never ends in /. Tested with / + 1022, 2046 and 2047 as + //b: the first ancestor stops before the //, no ancestor ends in /, and the usable list equals the old one.

4. More cases.

• stripFences: 200k-character cases with U+2029 and CRLF line ends.
• Directive lines: cases with U+2028 and U+2029.
• A hand-picked feed with several <enclosure> and <media:content> tags, type= before and after url=, the word url in text and data-url. It equals the old regex.
• Pre-existing quirk, unchanged: the media regex has a word boundary between - and url, so it reads data-url="…" as the url attribute. When it comes first in a tag, it wins over a later url=. The scanner keeps that, since this PR's promise is that results are unchanged. Counting only real url attributes would be a small behaviour change for a follow-up.

Checks for round 3:

• Changed files on their own: redos-174 (8 tests), xml-scan (5), public-input (4), frontmatter-lines (2), paths (2) and markdown-blocks-redos.test.tsx (2), all green.
• Then the affected packages one at a time: docs (3 files), schedules (11), agent-runtime (11), ui (17), api (40). Then core-ts without store.test.ts: 60 files, 661 tests. Then store.test.ts's better-sqlite3 slice: 156 tests, the slice that covers redirect matching. All green. Its D1 and Durable Object halves are left to CI, which ran them green on round 2.
• pnpm lint (0 errors), pnpm size:check and the typecheck of core-ts, schedules and api are green.

🤖 Generated with Claude Code